Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that all the settings are actually available. #52

Merged
merged 2 commits into from
May 14, 2020

Conversation

S-Dafarra
Copy link
Collaborator

@S-Dafarra S-Dafarra commented Feb 6, 2020

As it can be noticed from the osqp code some settings may not be available depending on the specific Cmake settings. With this PR we copy the same checks adding an error message in case some settings are not available.

I also added the time_limit setting (see #38).

In draft since I have to bump the version number, but there are other PR open at the moment.

@S-Dafarra S-Dafarra changed the title Chech that all the settings are actually avaliable. Check that all the settings are actually available. Feb 6, 2020
@S-Dafarra S-Dafarra marked this pull request as ready for review February 7, 2020 08:54
@S-Dafarra
Copy link
Collaborator Author

Rebased and bumped patch version. Removed from draft.

@GiulioRomualdi
Copy link
Member

Should we use a static_assert if the variable is not defined?

@S-Dafarra
Copy link
Collaborator Author

Should we use a static_assert if the variable is not defined?

I totally missed this 😁. The point of adding the error instead of the assert is that code would work anyway even if you are trying to use that quantity. With the static_assert the code would not compile even if I don't call that method at all right?

@GiulioRomualdi
Copy link
Member

Should we use a static_assert if the variable is not defined?

I totally missed this . The point of adding the error instead of the assert is that code would work anyway even if you are trying to use that quantity. With the static_assert the code would not compile even if I don't call that method at all right?

Right

@S-Dafarra
Copy link
Collaborator Author

Right

If we would use c++17 it would be easier to add it I guess, but I would avoid adding it for the moment. Let me rebase on top of devel though.

@GiulioRomualdi
Copy link
Member

Merging!

@GiulioRomualdi GiulioRomualdi merged commit 2eda1ea into robotology:devel May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants